longjob/src/main/java/org/zstack/longjob/LongJobManagerImpl.java (1)

463-472: 可考虑优化：减少重复数据库查询

在第 464-466 行查询了 state 和 jobName，但在第 469 行又执行了一次 findByUuid 获取完整的 LongJobVO。可以考虑在需要返回 VO 时直接查询完整对象，或者延迟到真正需要时再查询。

♻️ 建议的优化

     private void suspendLongJob(String uuid, ReturnValueCompletion<LongJobVO> completion) {
         Tuple t = Q.New(LongJobVO.class).eq(LongJobVO_.uuid, uuid).select(LongJobVO_.state, LongJobVO_.jobName).findTuple();
         LongJobState currentState = t.get(0, LongJobState.class);
         String jobName = t.get(1, String.class);
 
         if (currentState == LongJobState.Suspended) {
-            LongJobVO vo = dbf.findByUuid(uuid, LongJobVO.class);
-            completion.success(vo);
+            completion.success(dbf.findByUuid(uuid, LongJobVO.class));
             return;
         }

longjob/src/main/java/org/zstack/longjob/LongJobManagerImpl.java (1)

484-501: 与 cancelLongJob 相比缺少中间状态的问题已在之前的评审中提出。

当前实现在调用 job.suspend() 之前没有设置中间状态（如 Suspending），而 cancelLongJob 会在第 319 行先设置 Canceling 状态。虽然 changeState 内部使用了 synchronized(jobUuids.intern(uuid)) 进行同步，但这可能依赖于 suspend 操作的幂等性设计。

建议确认 suspend 操作是否设计为幂等的，或考虑添加 Suspending 中间状态以与 cancel 模式保持一致。

longjob/src/main/java/org/zstack/longjob/LongJobUtils.java (1)

172-189: 建议将 "longJobRecoverable" 提取为常量
Line 185 使用字符串字面量作为跨模块约定的 key，存在拼写不一致风险。建议提为常量（必要时可公开）以减少魔法值。

♻️ 建议修改

 public class LongJobUtils {
     private static final CLogger logger = Utils.getLogger(LongJobUtils.class);
+    public static final String LONG_JOB_RECOVERABLE_OPAQUE_KEY = "longJobRecoverable";
@@
-        Object recoverable = errorCode.getFromOpaque("longJobRecoverable");
+        Object recoverable = errorCode.getFromOpaque(LONG_JOB_RECOVERABLE_OPAQUE_KEY);

As per coding guidelines, ...

longjob/src/main/java/org/zstack/longjob/LongJobUtils.java (1)

182-186: 将 opaque key 抽成常量，避免魔法字符串
建议把 "longJobRecoverable" 提取为私有常量，降低拼写风险和复用成本。

♻️ 建议修改

 public class LongJobUtils {
     private static final CLogger logger = Utils.getLogger(LongJobUtils.class);
+    private static final String LONG_JOB_RECOVERABLE_OPAQUE_KEY = "longJobRecoverable";
@@
-        Object recoverable = errorCode.getFromOpaque("longJobRecoverable");
+        Object recoverable = errorCode.getFromOpaque(LONG_JOB_RECOVERABLE_OPAQUE_KEY);

longjob/src/main/java/org/zstack/longjob/LongJobManagerImpl.java (2)

487-493: 必须处理 suspended 返回值

当前实现忽略了 job.suspend() 的返回值 suspended，无论其为 true 还是 false 都会将状态变更为 Suspended。如果实现方返回 false 表示暂停未真正执行，这将导致状态与实际不一致。

建议仅在 suspended == true 时变更状态。

建议修复

 job.suspend(vo, new ReturnValueCompletion<Boolean>(completion) {
     `@Override`
     public void success(Boolean suspended) {
-        LongJobVO updatedVo = changeState(uuid, LongJobStateEvent.suspend);
-        logger.info(String.format("longjob [uuid:%s, name:%s] has been suspended", vo.getUuid(), vo.getName()));
-        completion.success(updatedVo);
+        if (Boolean.TRUE.equals(suspended)) {
+            LongJobVO updatedVo = changeState(uuid, LongJobStateEvent.suspend);
+            logger.info(String.format("longjob [uuid:%s, name:%s] has been suspended", vo.getUuid(), vo.getName()));
+            completion.success(updatedVo);
+        } else {
+            logger.info(String.format("longjob [uuid:%s, name:%s] suspend returned false, state unchanged", vo.getUuid(), vo.getName()));
+            completion.success(dbf.findByUuid(uuid, LongJobVO.class));
+        }
     }

---

484-486: 建议：考虑添加中间状态以保持一致性

对比 cancelLongJob 在调用 job.cancel() 前会先设置 Canceling 中间状态（Line 319），而 suspendLongJob 在调用 job.suspend() 前没有设置中间状态。这可能导致并发请求同时通过 Running 状态检查。

如果 suspend 操作设计为幂等的，当前实现可以接受；否则建议添加 Suspending 中间状态。

longjob/src/main/java/org/zstack/longjob/LongJobUtils.java (1)

182-190: 建议提取魔法值为常量并简化布尔判断逻辑。

根据编码规范，应避免使用魔法值。字符串 "longJobRecoverable" 建议定义为常量。同时，布尔判断可以使用 Boolean.TRUE.equals() 简化，更加简洁且能安全处理 null 和非 Boolean 类型。

♻️ 建议的重构

+    public static final String LONG_JOB_RECOVERABLE_KEY = "longJobRecoverable";
+
     /**
      * Check if an error is marked as recoverable in its opaque field.
      * Any business module can mark an error as recoverable by setting
      * "longJobRecoverable" to true in the error code's opaque field.
      * This allows the long job framework to automatically suspend instead of fail
      * for recoverable errors, enabling automatic retry after service restart.
      *
      * `@param` errorCode the error code to check
      * `@return` true if the error is marked as recoverable, false otherwise
      */
     private static boolean isRecoverableError(ErrorCode errorCode) {
-        // Check if error code has recoverable flag in opaque
-        // Any business module can set this flag to indicate the error is recoverable
-        Object recoverable = errorCode.getFromOpaque("longJobRecoverable");
-        if (recoverable instanceof Boolean && (Boolean) recoverable) {
-            return true;
-        }
-        return false;
+        Object recoverable = errorCode.getFromOpaque(LONG_JOB_RECOVERABLE_KEY);
+        return Boolean.TRUE.equals(recoverable);
     }

testlib/src/main/java/org/zstack/testlib/TestLibController.java (2)

327-327: 未使用的字段 cachedReader。

该字段已声明但从未被使用，可以移除。

♻️ 建议移除

     private static class CachedHttpServletRequest implements HttpServletRequest {
         private final HttpServletRequest delegate;
         private final String cachedBody;
-        private BufferedReader cachedReader;

---

517-523: 字符编码硬编码为 UTF-8，应使用请求的编码。

如果请求使用非 UTF-8 编码，这里的转换可能会导致内容损坏。此外，UTF-8 是 Java 保证支持的编码，UnsupportedEncodingException 永远不会抛出。

♻️ 建议改进

-        public CachedServletInputStream(String body) {
-            try {
-                this.bodyBytes = body != null ? body.getBytes("UTF-8") : new byte[0];
-            } catch (java.io.UnsupportedEncodingException e) {
-                throw new RuntimeException(e);
-            }
+        public CachedServletInputStream(String body) {
+            this.bodyBytes = body != null ? body.getBytes(java.nio.charset.StandardCharsets.UTF_8) : new byte[0];
         }

如需支持多种编码，可考虑将请求的 getCharacterEncoding() 传入此类。

testlib/src/main/java/org/zstack/testlib/TestLibController.java (4)

224-236: readLine() 会丢失换行符，导致请求体损坏
小体积分支用 readLine() 会丢弃换行符，多行 JSON/文本会被破坏，且与大体积分支行为不一致。

✅ 建议修复（统一用字符缓冲区读取）

-            if (contentLength > 0 && contentLength < 1024 * 1024) {
-                // For small bodies (< 1MB), use StringBuilder with known capacity
-                StringBuilder sb = new StringBuilder(contentLength);
-                String line;
-                while ((line = reader.readLine()) != null) {
-                    sb.append(line);
-                    if (sb.length() > maxSize) {
-                        throw new IOException("Request body exceeds maximum size: " + maxSize + " bytes");
-                    }
-                }
-                return sb.toString();
-            } else {
-                // For larger or unknown size, read in chunks with size tracking
-                StringBuilder sb = new StringBuilder(8192);
-                char[] buffer = new char[8192];
-                int charsRead;
-                long totalRead = 0;
-                while ((charsRead = reader.read(buffer)) != -1) {
-                    totalRead += charsRead;
-                    if (totalRead > maxSize) {
-                        throw new IOException("Request body exceeds maximum size: " + maxSize + " bytes");
-                    }
-                    sb.append(buffer, 0, charsRead);
-                }
-                return sb.toString();
-            }
+            // Use char buffer to preserve all characters including newlines
+            int initialCapacity = (contentLength > 0 && contentLength < 1024 * 1024) ? contentLength : 8192;
+            StringBuilder sb = new StringBuilder(initialCapacity);
+            char[] buffer = new char[8192];
+            int charsRead;
+            long totalRead = 0;
+            while ((charsRead = reader.read(buffer)) != -1) {
+                totalRead += charsRead;
+                if (totalRead > maxSize) {
+                    throw new IOException("Request body exceeds maximum size: " + maxSize + " bytes");
+                }
+                sb.append(buffer, 0, charsRead);
+            }
+            return sb.toString();

---

258-266: Content-Length 使用字符数会导致长度不准确
responseBody.length() 是字符数，非字节数；包含非 ASCII 时会错误。

✅ 建议修复（按 UTF‑8 字节长度）

-            headers.setContentLength(responseBody != null ? responseBody.length() : 0);
-            
-            HttpEntity<String> entity = new HttpEntity<>(responseBody != null ? responseBody : "", headers);
+            String body = responseBody != null ? responseBody : "";
+            byte[] bodyBytes = body.getBytes(java.nio.charset.StandardCharsets.UTF_8);
+            headers.setContentLength(bodyBytes.length);
+            
+            HttpEntity<String> entity = new HttpEntity<>(body, headers);

---

280-288: 错误 JSON 需要转义，避免生成无效 JSON
String.format 直接拼接 error.getMessage() 时，遇到引号/反斜杠/换行会破坏 JSON。建议至少做转义（或改用 JSON 序列化工具）。

✅ 建议修复（最小转义 + 正确 Content-Length）

-            String errorBody = String.format("{\"error\":\"%s\"}", error.getMessage());
+            String errorMsg = error.getMessage() != null ? error.getMessage() : "Unknown error";
+            errorMsg = errorMsg.replace("\\", "\\\\")
+                               .replace("\"", "\\\"")
+                               .replace("\n", "\\n")
+                               .replace("\r", "\\r")
+                               .replace("\t", "\\t");
+            String errorBody = String.format("{\"error\":\"%s\"}", errorMsg);
             HttpHeaders headers = new HttpHeaders();
             headers.setContentType(MediaType.APPLICATION_JSON);
             headers.set(RESTConstant.TASK_UUID, taskUuid);
-            headers.setContentLength(errorBody.length());
+            byte[] bodyBytes = errorBody.getBytes(java.nio.charset.StandardCharsets.UTF_8);
+            headers.setContentLength(bodyBytes.length);

---

499-499: getOutputStream() 直接抛异常会使部分处理器测试失败
若被测处理器使用 getOutputStream() 写响应，将导致测试异常。建议提供基于 ByteArrayOutputStream 的实现或明确兼容方式。

testlib/src/main/java/org/zstack/testlib/TestLibController.java (5)

247-257: 小体积分支仍会丢失换行符

reader.readLine() 会丢弃换行符，导致多行 JSON/文本被破坏，且与大体积分支行为不一致。建议统一使用字符缓冲读取。

---

281-286: Content-Length 应使用字节长度

responseBody.length() 是字符数，非字节数。对非 ASCII 内容会导致 Content-Length 不准确。

---

301-309: 错误回调 JSON 未做转义

String.format("{\"error\":\"%s\"}", error.getMessage()) 未转义特殊字符，可能生成无效 JSON。建议使用 JSON 序列化。

---

378-403: 异步线程仍依赖原始 HttpServletRequest

后台线程通过 delegate 访问请求对象（如 getHeader()/getRequestURI()），而响应已提交后容器不保证请求对象仍可用，可能导致不稳定。建议在返回前快照所需字段并改用独立对象传递。

---

531-531: MockHttpServletResponse 不支持 getOutputStream

若被测处理器使用 getOutputStream()，这里会抛异常导致测试失败。建议提供基于缓冲的实现或更明确的降级策略。

longjob/src/main/java/org/zstack/longjob/LongJobManagerImpl.java (1)

487-493: 必须处理 suspended 返回值，否则会错误标记为 Suspended。
Line 489-492 直接切换到 Suspended，忽略了 suspended 的实际结果，可能导致状态与真实暂停结果不一致。

🔧 建议修正

         job.suspend(vo, new ReturnValueCompletion<Boolean>(completion) {
             `@Override`
             public void success(Boolean suspended) {
-                LongJobVO updatedVo = changeState(uuid, LongJobStateEvent.suspend);
-                logger.info(String.format("longjob [uuid:%s, name:%s] has been suspended", vo.getUuid(), vo.getName()));
-                completion.success(updatedVo);
+                if (Boolean.TRUE.equals(suspended)) {
+                    LongJobVO updatedVo = changeState(uuid, LongJobStateEvent.suspend);
+                    logger.info(String.format("longjob [uuid:%s, name:%s] has been suspended", vo.getUuid(), vo.getName()));
+                    completion.success(updatedVo);
+                } else {
+                    logger.info(String.format("longjob [uuid:%s, name:%s] suspend returned false; keep state unchanged",
+                            vo.getUuid(), vo.getName()));
+                    completion.success(dbf.findByUuid(uuid, LongJobVO.class));
+                }
             }

longjob/src/main/java/org/zstack/longjob/LongJobFactory.java (1)

10-15: 为新增接口方法补充 Javadoc 说明
Line 14 新增的 supportSuspend 建议补充英文 Javadoc，明确语义与返回值，便于 API 文档与维护一致。As per coding guidelines, please add Javadoc for interface methods.

♻️ 建议修改

+    /**
+     * Whether the long job supports suspend operation.
+     */
     boolean supportSuspend(String jobName);